Skip to content

Always allow localhost CORS requests #1398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 26, 2020

Conversation

andrewn
Copy link
Member

@andrewn andrewn commented Apr 19, 2020

In order to allow developers to run only the client app code on their machines (see #1348), they need to be able to send requests from the app running on localhost to a remote instance of the Editor API.

Our CORS restrictions currently prohibit localhost in production so this PR removes that restriction.

I've thought about the security implications of this, and I think it's OK as the API will still require a user to be authenticated before doing anything.

It might be an idea to parallel deploy to a staging server i.e. staging.editor.p5js.org and only allow localhost CORS there to protect developers from accidentally doing anything weird. We'd also probably want a copy of mongo too?

--

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)

@catarak
Copy link
Member

catarak commented Apr 29, 2020

Awesome!! Agree that maybe it would make sense to only allow localhost from a staging server. Maybe this could be configured with an environment variable then?

@andrewn
Copy link
Member Author

andrewn commented May 3, 2020

I've added the CORS_ALLOW_LOCALHOST env var to handle this. When set to the string "true" it will be enabled.

@catarak
Copy link
Member

catarak commented May 22, 2020

I've added the CORS_ALLOW_LOCALHOST env var to handle this. When set to the string "true" it will be enabled.

Thank you!! I think you didn't push this change as I'm only seeing one commit on this PR?

@andrewn
Copy link
Member Author

andrewn commented May 26, 2020

@catarak Sorry. Pushed it now!

@catarak
Copy link
Member

catarak commented May 26, 2020

perfect :) i'll add this env to the staging server!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants